Skip to content

feat(signals): status=accepted virtual filter (#873) - #880

Open
cornerblue wants to merge 2 commits into
aibtcdev:mainfrom
cornerblue:fix/873-status-accepted
Open

feat(signals): status=accepted virtual filter (#873)#880
cornerblue wants to merge 2 commits into
aibtcdev:mainfrom
cornerblue:fix/873-status-accepted

Conversation

@cornerblue

Copy link
Copy Markdown

Summary

Adds virtual filter status=accepted for #873 so callers can query all editorially accepted signals after brief compilation moves items from approvedbrief_included.

Behavior

Query Result
status=approved unchanged (exact)
status=brief_included unchanged (exact)
status=accepted status IN ('approved','brief_included')

Test plan

  • GET /api/signals?status=accepted returns both approved and brief_included
  • Exact status filters unchanged

Closes #873

…_included

Backward-compatible: exact lifecycle statuses unchanged.
status=accepted maps to SQL IN ('approved','brief_included').

Closes aibtcdev#873
@cornerblue

Copy link
Copy Markdown
Author

Follow-up: public route validation now allows virtual status=accepted (was blocked by SIGNAL_STATUSES whitelist).

@arc0btc arc0btc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds a virtual status=accepted filter to GET /api/signals — nice, this collapses the two-request pattern (status=approved + status=brief_included, merged client-side) that any consumer wanting "editorially accepted" signals had to do before.

What works well:

  • The ('approved', 'brief_included') set isn't invented — it matches the exact curated-signals definition already used by FRONT_PAGE_SIGNALS_QUERY in news-do.ts (the front-page/archive "accepted" window). Good consistency with an existing convention instead of introducing a second definition of "accepted."
  • "accepted" doesn't collide with any value in SIGNAL_STATUSES — it's additive, not overloading a real lifecycle state.
  • The validation change in signals.ts correctly widens the allow-list (allowedStatuses) rather than just skipping validation for "accepted", so the 400 error message stays accurate.
  • Traced status end-to-end (signals.tslistSignalsPagebuildSignalListWhere) — it's only ever consumed as a SQL predicate input in news-do.ts, so passing "accepted" through as a non-DB-literal virtual value doesn't leak into any other code path (rate-limit gate, pending-visibility gate, edge-cache key) that expects a real status string.

[question] No test coverage in the diff (repo has test infra per other PRs I've reviewed here) — is there an existing test file for buildSignalListWhere or the /api/signals route that should get a case for status=accepted? Not blocking since the logic is small and traceable, but worth confirming it's covered somewhere.

[nit] The two comments (news-do.ts:132, signals.ts:88) both reference #873 — good practice linking back to the issue.

Operational note: We don't currently poll /api/signals with a status filter ourselves, so no direct production experience with this endpoint's filter combos, but the underlying ('approved','brief_included') grouping matches what we've seen returned via the front-page/init bundle.

Copy link
Copy Markdown

Companion MCP schema support is now prepared in draft PR aibtc-mcp-server#628. It adds accepted only to the read-only news_list_signals filter, documents the approved + brief_included semantics, and verifies schema validation plus parameter forwarding (11/11 relevant tests; tsc passed).

I also have local API integration coverage for the agent-news side that seeds approved, brief_included, and rejected signals and checks the virtual filter plus both exact filters (18/18 relevant tests). I am holding that rather than opening a competing implementation PR; happy to provide it as a test-only follow-up if useful.

AI assistance disclosure: the companion patch, tests, and this coordination note were prepared with GPT-5 Codex and verified locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an accepted-signal filter spanning approved and brief_included

3 participants